Improve SiloCast remote UI and reliability - #253
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (15)
Included review availability: 0 reviews are currently available. Your included PR review attempts over the past 7 days set your current allowance at 1 review per hour. 📝 WalkthroughWalkthroughThe change adds canonical server identity matching, serialized cast commands, optimistic volume reconciliation, retained TV volume state, hardware volume control, adaptive remote layouts, and simplified media service startup behavior. ChangesCast control and identity
Estimated code review effort: 5 (Critical) | ~90 minutes Merge Risk: ⚪ Minimal · up to The PR improves remote controls, volume synchronization, server identity handling, and service reliability; no actionable merge-blocking risk remains after normal checks and review. Sequence Diagram(s)sequenceDiagram
participant MainActivity
participant SiloCastController
participant RemoteVolumeReconciler
participant ControlTransport
participant TV
MainActivity->>SiloCastController: stepVolumeOptimistic(step)
SiloCastController->>RemoteVolumeReconciler: record requested volume
SiloCastController->>ControlTransport: enqueue volume command
ControlTransport->>TV: write control frame
TV-->>SiloCastController: publish playback state
SiloCastController->>RemoteVolumeReconciler: reconcile inbound volume
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4f963d2673
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (currentVolume != null) { | ||
| val clamped = currentVolume.coerceIn(0.0, 1.0) | ||
| muted = clamped <= SILENT_VOLUME | ||
| if (!muted) retainedVolume = clamped |
There was a problem hiding this comment.
When handling a mute or unmute command, recordMuted updates muted immediately, but the state provider then calls resolve with the MediaController's current volume. If that asynchronous controller has not reflected the just-issued volume change yet, these lines immediately reverse the explicit state—for example, recordMuted(true, 0.42) followed by resolve(0.42) reports unmuted. The TV consequently sends a contradictory acknowledgement and the remote can flicker or require another interaction; retain the requested mute state until the controller observes the corresponding volume.
Useful? React with 👍 / 👎.
| message.state.copy( | ||
| volume = volumeReconciler.reconcile(message.state.volume, now), | ||
| ) |
There was a problem hiding this comment.
When the user toggles mute while an older periodic state frame is already in flight, this copies the inbound isMuted unchanged even though setMuted just updated it optimistically; only volume is reconciled. The stale frame can therefore undo the toggle, and a quick second tap sends the same command again instead of unmuting. Track pending mute requests or otherwise preserve the optimistic isMuted value until its acknowledgement arrives, as is already done for volume.
Useful? React with 👍 / 👎.
1 participant
Summary
Validation
./gradlew test :androidApp:assembleDebug :androidTvApp:assembleDebugSiloCastMediaSessionStarterTestregression coverageSummary by CodeRabbit
New Features
Bug Fixes